Github action to periodically `cargo update` to keep dependencies current #110805

pitaj · 2023-04-25T16:02:52Z

Opens a PR periodically with the results of cargo update. If an unmerged PR for the branch cargo_update already exists, it will edit then reopen it if necessary.

~~This also uses cargo-upgrades to provide a list of available major upgrades in the PR body.~~

It includes the list of changes output by cargo update in the commit message and PR body. Note that this output is currently sub-optimal due to rust-lang/cargo#9408, but if updates are made more regularly that is less likely to show up.

Example PR: pitaj#2
Example action run: https://github.com/pitaj/rust/actions/runs/5035731903
Prior discussion: https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/dependabot.20updates.3F

Up for discussion:

What period do we want? Currently weekly
What user should it use? Currently "Github Actions"
Do we need the extra security of provided by executing cargo update and cargo-upgrades in a separate job?
If not I can simplify it to not need artifacts.
PR message wording
PR should probably always be rollup=always?
What branch should it use?
What should it do if no updates are available? Currently fails the job on empty commit
Should the yml file live in src/ci instead of directly under workflows?
~~Is using the latest nightly toolchain enough to ensure compatibility with Cargo.lock and Cargo.tomls in master?~~
Now pulls the bootstrap version from stage0.json

r? infra

Mark-Simulacrum · 2023-05-18T02:01:51Z

Taking a brief look, some initial thoughts:

Can we avoid creating the branch in this repository? That's somewhat noisy for others and in general seems like it would be nice to avoid (I think it slightly increases likelihood that the PR created would have access to secrets, for example, even pre-approval).
Can we filter out or skip listing any submodule/subtree dependencies? We generally don't want to touch upstream lock files I think -- that makes syncs harder and similar automation ideally also runs there if we want something like this.
Following from the last point, I'm not sure how useful the list of pending major version upgrades is. If someone has bandwidth to work on those, it seems like they could also run the command themselves. So I'd lean towards omitting them. (I initially thought that was the delta list, which might've been more helpful).

pitaj · 2023-05-18T03:21:04Z

Thanks for taking a look.

Can we avoid creating the branch in this repository?

I'm not sure if that's possible, but I will look into it. Are you thinking rust-lang-ci/rust instead?

Following from the last point, I'm not sure how useful the list of pending major version upgrades is. If someone has bandwidth to work on those, it seems like they could also run the command themselves.

Yeah that's fair. I thought it would be useful to see them periodically, but maybe this should be handled separately. Open an issue for each major upgrade instead. Marking them as E-easy would invite community contributions. I'll note "filter out sub deps" for that.

I initially thought that was the delta list, which might've been more helpful

Good idea. I'll look into parsing Cargo.lock and including the set of changes in the commit message and PR body.

Mark-Simulacrum · 2023-05-18T12:33:53Z

I'm not sure if that's possible, but I will look into it. Are you thinking rust-lang-ci/rust instead?

I think that would work.

Good idea. I'll look into parsing Cargo.lock and including the set of changes in the commit message and PR body.

I think the cargo update command should print this as it runs? Can we just use that?

pitaj · 2023-05-21T04:21:24Z

I've removed the cargo-upgrades stuff and added the cargo-upgrade log to the PR and commit message instead.

It looks like pushing to a different repository is not at all simple. It requires a separate auth token and everything has to be done manually. It would significantly complicate things. Can you explain more about what your concern is regarding "noisiness"? As for secrets, I have to admit I'm not well acquainted with the security model there. Why do you think opening a PR from a branch in this repository is more risky?

.github/workflows/dependencies.yml

Mark-Simulacrum · 2023-06-10T12:00:02Z

.github/workflows/dependencies.yml

+          git add ./Cargo.lock
+          git commit --no-verify --file=commit.txt
+
+      - name: edit and reopen existing pull request


Is this addressing the case where the PR was already merged? Or where the PR was closed without merging? I think my current feeling is that we should not push commits or otherwise run updates until the previous PR merges. With at least current bors, reusing a single PR across possible r+'s is going to be more confusing and I think more likely to run into us not merging it in time before it gets bumped again by the automation here.

Can we bail out here if a PR is already open?

The behavior I had is this:

If PR was closed without merging, reopen, edit, and push

If PR was merged, push and open new PR

If PR is already open, edit and push

My thinking was that someone will probably check in on the PR when they have time, and it shouldn't be a week outdated when they check it. However, I was operating under and assumption of the "Github merge button" and hadn't considered bors, so I'll have to change it to never reopen a closed PR.

So the new behavior will be:

If PR is S-waiting-on-bors, skip

If PR was closed for any reason, push and open new PR

If PR is already open, push and edit

Mark-Simulacrum · 2023-06-10T12:01:24Z

It looks like pushing to a different repository is not at all simple. It requires a separate auth token and everything has to be done manually. It would significantly complicate things. Can you explain more about what your concern is regarding "noisiness"? As for secrets, I have to admit I'm not well acquainted with the security model there. Why do you think opening a PR from a branch in this repository is more risky?

When folks clone and/or git pull in this repo, any new branches that get touched are also pulled. I think I missed that this PR is always(?) updating a single branch, which doesn't seem that bad, I think that addresses that concern for me.

As for secrets, I have to admit I'm not well acquainted with the security model there. Why do you think opening a PR from a branch in this repository is more risky?

Hm. So thinking more about this in practice I expect these PRs to get r+'d pretty quickly, so I don't think there's value in trying to go to a different repository. I think GitHub Actions exposes secrets configured in this repository to all branches in the repo, which it wouldn't do externally, but I think the risk/benefit here is pretty minimal.

Left a few inline comments but I think we can squash commits and move forward once the comments are resolved.

Mark-Simulacrum · 2023-06-10T12:01:34Z

@rustbot author

pitaj · 2023-06-10T16:29:22Z

I think I missed that this PR is always(?) updating a single branch, which doesn't seem that bad, I think that addresses that concern for me.

Yes, it's always the cargo_update branch. And at most, that branch will have one commit relative to master with only one file changed, so it should have practically zero impact on clones or fetches.

pitaj · 2023-06-10T18:05:49Z

Alright I've addressed those issues and rebased

@rustbot ready

Mark-Simulacrum · 2023-06-10T19:11:05Z

.github/workflows/dependencies.yml

+          git commit --no-verify --file=commit.txt
+
+      - name: push
+        run: git push --no-verify -f -u origin cargo_update


I think this step needs to be moved after checking for existing PRs -- my thinking is that we should refuse to update a PR. We might be able to alter that to something like "no updates if labeled S-waiting-on-bors", but that might be too complex.

If you don't want to update a PR at all, that's fine. But if your concern is just bors, the behavior I have here should cover that:

The whole thing is skipped when the existing PR is open and marked S-waiting-on-bors

When the existing PR is closed, pushes don't update the existing PR, so we push and open a new PR

Only when the existing PR is open and not S-waiting-on-bors, push and edit that PR

That said, moving the push after the edit or open would make this less reliant on that "pushing to a closed GitHub PR doesn't update it" behavior. I'll go ahead and make that change and retest that everything works.

Ah, I missed that block earlier in the file -- happy to merge with or without that change. I think pushing to a closed PR makes it impossible to re-open it, so I think making this change is probably good, but non-critical. (We almost certainly will rarely or never be closing these PRs).

Okay wasn't able to get push-after-open to work, but if that's alright with you, we should be good to go.

pitaj · 2023-06-10T20:02:04Z

Alright it looks like using cargo from bootstrap beta is not gonna work as well as I'd hoped. I'll try switching back to nightly cargo.

Edit: ah actually maybe I just need RUSTC_BOOTSTRAP=1

- Keep Cargo.lock dependencies current - Presents output from `cargo update` in commit and PR - Edit existing open PR, otherwise open a new one - Skip if existing open PR is S-waiting-on-bors

Mark-Simulacrum · 2023-06-16T23:36:33Z

@bors r+ rollup

Thanks! I think we can go ahead and land this, and try it out in practice. If we encounter issues can always back out and try again.

bors · 2023-06-16T23:36:35Z

📌 Commit 6d3ff10 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

Github action to periodically `cargo update` to keep dependencies current Opens a PR periodically with the results of `cargo update`. If an unmerged PR for the branch `cargo_update` already exists, it will edit then reopen it if necessary. ~~This also uses [`cargo-upgrades`](https://gitlab.com/kornelski/cargo-upgrades) to provide a list of available major upgrades in the PR body.~~ It includes the list of changes output by `cargo update` in the commit message and PR body. Note that this output is currently sub-optimal due to rust-lang/cargo#9408, but if updates are made more regularly that is less likely to show up. Example PR: pitaj#2 Example action run: https://github.com/pitaj/rust/actions/runs/5035731903 Prior discussion: https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/dependabot.20updates.3F Up for discussion: - What period do we want? Currently weekly - What user should it use? Currently "Github Actions" - Do we need the extra security of provided by executing `cargo update` and `cargo-upgrades` in a separate job? If not I can simplify it to not need artifacts. - PR message wording - PR should probably always be `rollup=always`? - What branch should it use? - What should it do if no updates are available? Currently fails the job on empty commit - Should the yml file live in `src/ci` instead of directly under workflows? - ~~Is using the latest nightly toolchain enough to ensure compatibility with `Cargo.lock` and `Cargo.toml`s in master?~~ Now pulls the bootstrap version from stage0.json r? infra

…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#110805 (Github action to periodically `cargo update` to keep dependencies current) - rust-lang#112435 (Allow overwriting the sysroot compile flag via --rustc-args) - rust-lang#112610 (Bump stdarch) - rust-lang#112619 (Suggest bumping download-ci-llvm-stamp if the build config for llvm changes) - rust-lang#112738 (make ice msg "Unknown runtime phase" a bit nicer) r? `@ghost` `@rustbot` modify labels: rollup

klensy · 2023-06-18T09:34:52Z

I guess it's failed #112746 (comment)?

pitaj · 2023-06-18T13:49:22Z

I guess it's failed #112746 (comment)?

Yeah we're aware. Bot doesn't have enough privileges to push to that branch. I'm researching what exactly needs to be done to resolve that.

I'm not sure why it showed up in the PR, but we may want to look into excluding it from log analysis.

pitaj added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label Apr 25, 2023

rustbot assigned shepmaster Apr 25, 2023

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 25, 2023

Mark-Simulacrum assigned Mark-Simulacrum and unassigned shepmaster May 10, 2023

This comment was marked as outdated.

Sign in to view

Mark-Simulacrum reviewed Jun 10, 2023

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 10, 2023

pitaj force-pushed the master branch from ec61a23 to 508c84b Compare June 10, 2023 18:04

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 10, 2023

Mark-Simulacrum reviewed Jun 10, 2023

View reviewed changes

pitaj force-pushed the master branch from 508c84b to 2b8d706 Compare June 10, 2023 19:33

pitaj force-pushed the master branch 3 times, most recently from 23c25bf to 289ae56 Compare June 10, 2023 20:44

run cargo update weekly

6d3ff10

- Keep Cargo.lock dependencies current - Presents output from `cargo update` in commit and PR - Edit existing open PR, otherwise open a new one - Skip if existing open PR is S-waiting-on-bors

pitaj force-pushed the master branch from 289ae56 to 6d3ff10 Compare June 10, 2023 20:47

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 16, 2023

matthiaskrgr mentioned this pull request Jun 17, 2023

Rollup of 8 pull requests #112730

Closed

This was referenced Jun 17, 2023

Rollup of 6 pull requests #112745

Closed

Rollup of 5 pull requests #112746

Merged

bors merged commit be0f3bd into rust-lang:master Jun 17, 2023

rustbot added this to the 1.72.0 milestone Jun 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Github action to periodically `cargo update` to keep dependencies current #110805

Github action to periodically `cargo update` to keep dependencies current #110805

pitaj commented Apr 25, 2023 •

edited

Loading

Mark-Simulacrum commented May 18, 2023

pitaj commented May 18, 2023 •

edited

Loading

Mark-Simulacrum commented May 18, 2023

This comment was marked as outdated.

pitaj commented May 21, 2023

Mark-Simulacrum Jun 10, 2023

pitaj Jun 10, 2023

Mark-Simulacrum commented Jun 10, 2023

Mark-Simulacrum commented Jun 10, 2023

pitaj commented Jun 10, 2023

pitaj commented Jun 10, 2023

Mark-Simulacrum Jun 10, 2023

pitaj Jun 10, 2023 •

edited

Loading

Mark-Simulacrum Jun 10, 2023

pitaj Jun 10, 2023

pitaj commented Jun 10, 2023 •

edited

Loading

Mark-Simulacrum commented Jun 16, 2023

bors commented Jun 16, 2023

klensy commented Jun 18, 2023

pitaj commented Jun 18, 2023

Github action to periodically cargo update to keep dependencies current #110805

Github action to periodically cargo update to keep dependencies current #110805

Conversation

pitaj commented Apr 25, 2023 • edited Loading

Mark-Simulacrum commented May 18, 2023

pitaj commented May 18, 2023 • edited Loading

Mark-Simulacrum commented May 18, 2023

This comment was marked as outdated.

pitaj commented May 21, 2023

Mark-Simulacrum Jun 10, 2023

Choose a reason for hiding this comment

pitaj Jun 10, 2023

Choose a reason for hiding this comment

Mark-Simulacrum commented Jun 10, 2023

Mark-Simulacrum commented Jun 10, 2023

pitaj commented Jun 10, 2023

pitaj commented Jun 10, 2023

Mark-Simulacrum Jun 10, 2023

Choose a reason for hiding this comment

pitaj Jun 10, 2023 • edited Loading

Choose a reason for hiding this comment

Mark-Simulacrum Jun 10, 2023

Choose a reason for hiding this comment

pitaj Jun 10, 2023

Choose a reason for hiding this comment

pitaj commented Jun 10, 2023 • edited Loading

Mark-Simulacrum commented Jun 16, 2023

bors commented Jun 16, 2023

klensy commented Jun 18, 2023

pitaj commented Jun 18, 2023

Github action to periodically `cargo update` to keep dependencies current #110805

Github action to periodically `cargo update` to keep dependencies current #110805

pitaj commented Apr 25, 2023 •

edited

Loading

pitaj commented May 18, 2023 •

edited

Loading

pitaj Jun 10, 2023 •

edited

Loading

pitaj commented Jun 10, 2023 •

edited

Loading